-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[manylinux2014] arm64 patchelf alignment fix #739
[manylinux2014] arm64 patchelf alignment fix #739
Conversation
Huh, Travis is failing on a curl call: Seems to be happening in some of the other PRs here too. Additionally the PPC builds are being detected as stalled, and that seems to be happening for the past week or so:
|
@reaperhulk , @mattip , could I get a review on this? |
It looks good to me, but is a bit hard to verify with a failing arm64 build. I also see problems with ppc64le on another repo. |
@mattip, can we kick the build again? That curl error is related to ephemeral port starvation, so may be intermittent, not sure if this is a more widespread travis problem or just got unlucky. |
304685d
to
8ac9c18
Compare
@mattip, just force pushing restarted travis, looks like ppcle is having odd timeout issues, no clue as to why, but Arm64, x86_64, and s390x are passing |
That's quite a patch -- I take it the change we want was squashed up in all those other changes? That's unfortunate... |
@reaperhulk, I'm happy to iterate on it, I can try cherry-picking just the alignment fix to see if it works too. Though my experience is not great with that going cleanly. |
It'd be good to try I think. If it proves unworkable we can always change our minds. |
8ac9c18
to
52838ed
Compare
@reaperhulk , @mattip , cherry-picking worked with just a minor merge issue. The resulting binary appears to be generating the proper alignments. Re-building the container to do a full test on a wheel. |
Internal testing on the pyca/cryptography shows it is generating binaries with the proper alignments on Arm. Still seeing ppcle failing with timeouts, though all other builds are working. |
The goal of this PR is to finx the ARM64 images, so maybe it can just go in as-is, without the ppc64le builds. They have been failing for the past week. |
Much easier to review! Could you add some text to make it clear that this patch can be backed out when patchelf is updated? Just in case someone else ends up doing that work. Otherwise this LGTM (although I am not a manylinux contributor so take my opinion with whatever sized grain of salt you desire) |
…4 machines with 64k pages. - Based on NixOS/patchelf@0470d69 Can be removed once a new version of patchelf is released containing the above commit.
…ix patch to 0.11 release.
52838ed
to
470f25c
Compare
Ok, comments added. I posted a question to the TravisCI mailing list about this PPC issue, seems odd it happens during a yum install. This fix is primarily applicable to Arm64, which has multiple page sizes in use in the wild. |
@mattip, anything else needed for this? I do not believe that the PPCLE build is necessary for a fix related to Arm64. |
@geoffreyblake you need to ping someone who can merge. |
@auvipy might be a good candidate here 😄 |
Bug reports for Manylinux2014_aarch64 wheels show that they contain bad alignment for Arm distros that use 64k pagesize. This updates patchelf 0.11 to include recent patches that force Arm64 page alignment to 64k regardless of the base system used for building.
Tested locally with the cffi project, resulting wheel and binary after performing audit wheel uses 64k alignment. Tested wheel on Centos8 on Arm64 and imported cffi with no issue.
Solves Issue: #735